-
Notifications
You must be signed in to change notification settings - Fork 3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix basic auth credentials in --extra-index-urls when using multiple credentials for same domain #10033
Conversation
If |
I think this is useful in case of 401 where url credentials are not (directly) available (like url forwarding by the server to a specific file). Also, to be honest, as this is my first PR here, I've tried to minimize the scope of my change. Feel free to improve it if anything. thank you |
Yeah, you’re right, The logic change looks correct to me, although some refactors would be best; we can get to that later. First you’ll need to fix the tests. |
Thank you for the feedback. I will fix the tests soon. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, assuming tests pass.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is restructuredText so double backticks are needed.
thank you for your helpful review @uranusjr |
Changes approved and tests are passing. Is it ready to be merged ? |
That is a very interesting feature. It would help us too Any idea of when it will be merged ? Thank you |
Whenever one of the maintainers (who are almost exclusively volunteers) come around to taking a look at this. |
Fix proposal for issue that is pending. Close #3931.